[SPARK-2052] [SQL] Add optimization for CaseConversionExpression's. by ueshin · Pull Request #990 · apache/spark

ueshin · 2014-06-06T04:57:44Z

Add optimization for CaseConversionExpression's.

AmplabJenkins · 2014-06-06T04:57:49Z

Merged build triggered.

AmplabJenkins · 2014-06-06T05:41:43Z

Merged build started.

AmplabJenkins · 2014-06-06T06:55:32Z

Merged build finished. All automated tests passed.

AmplabJenkins · 2014-06-06T06:55:32Z

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15495/

rxin · 2014-06-09T05:24:00Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala

Just curious - do you have a use case for this rule? I wonder how often it happens in practice.

Hi,
I just cared that the conversions were used redundantly like UPPER(LOWER(str)), which would be a mistake in most case, though.

Do you actually have queries that trigger this?

Just use UPPER(LOWER(str)) or UPPER(UPPER(str)), or something like those.

rxin · 2014-06-09T06:16:48Z

This looks good to me. Can you add some unit tests for collapsing case statements?

ueshin · 2014-06-09T06:48:34Z

Thank you for your comments.
I added some tests.

AmplabJenkins · 2014-06-09T06:52:50Z

Merged build triggered.

AmplabJenkins · 2014-06-09T08:01:03Z

Merged build started.

AmplabJenkins · 2014-06-09T09:14:58Z

Merged build finished. All automated tests passed.

AmplabJenkins · 2014-06-09T09:14:59Z

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15561/

marmbrus · 2014-06-09T19:12:04Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala

This case would be more generally handled by constant folding. Can you override foldable in these expressions to be true if the child is foldable? Also please fix the toString methods.

Fixed the two points.
Should I remove the mentioned lines?

Yes please.
On Jun 9, 2014 8:16 PM, "Takuya UESHIN" notifications@github.com wrote:

In
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala:

@@ -154,6 +155,10 @@ object NullPropagation extends Rule[LogicalPlan] {
case left :: Literal(null, _) :: Nil => Literal(null, e.dataType)
case _ => e
}

case e: CaseConversionExpression => e.children match {

Fixed the two points.
Should I remove the mentioned lines?

—
Reply to this email directly or view it on GitHub
https://github.com/apache/spark/pull/990/files#r13576056.

Ok, I do.
BTW, I noticed that there are some Expressions in the same situation like UnaryMinus, Cast, or Not, ( or BinaryArithmetic, BinaryComparison, StringRegexExpression, too ? )
I think they also can be handled by constant folding.
What do you think?

Good catch! Can you double check that constant folding works as expected and then remove the unneeded rules?

AmplabJenkins · 2014-06-10T03:12:49Z

Merged build triggered.

AmplabJenkins · 2014-06-10T05:08:04Z

Merged build started.

AmplabJenkins · 2014-06-10T05:08:07Z

Merged build finished.

AmplabJenkins · 2014-06-10T05:08:07Z

Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15597/

AmplabJenkins · 2014-06-10T07:32:49Z

Merged build triggered.

ueshin · 2014-06-10T07:35:15Z

@marmbrus I checked and remove the unneeded rules from NullPropagation.
Could you please check the changes?

AmplabJenkins · 2014-06-10T07:47:23Z

Merged build started.

AmplabJenkins · 2014-06-10T07:47:39Z

Merged build finished.

AmplabJenkins · 2014-06-10T07:47:40Z

Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15606/

marmbrus · 2014-06-11T05:18:07Z

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ConstantFoldingSuite.scala

This is only foldable because both values are literals. if you make one of them an attribute (e.g., 'a.int) then it won't fold anymore and we'll need the NullPropagation rule still.

Ah, I see, that's right!
I'll move them back.

AmplabJenkins · 2014-06-11T06:37:05Z

Merged build triggered.

AmplabJenkins · 2014-06-11T06:37:15Z

Merged build started.

AmplabJenkins · 2014-06-11T07:55:55Z

Merged build finished.

AmplabJenkins · 2014-06-11T07:55:55Z

Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15671/

marmbrus · 2014-06-11T19:06:01Z

Hey, this isn't mergable anymore. Mind rebasing?

…move the unneeded rules from NullPropagation.

ueshin · 2014-06-11T23:29:52Z

Rebased, thanks.

AmplabJenkins · 2014-06-11T23:32:06Z

Merged build triggered.

AmplabJenkins · 2014-06-11T23:32:14Z

Merged build started.

AmplabJenkins · 2014-06-12T00:54:47Z

Merged build finished. All automated tests passed.

AmplabJenkins · 2014-06-12T00:54:47Z

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15693/

Add optimization for `CaseConversionExpression`'s. Author: Takuya UESHIN <ueshin@happy-camper.st> Closes #990 from ueshin/issues/SPARK-2052 and squashes the following commits: 2568666 [Takuya UESHIN] Move some rules back. dde7ede [Takuya UESHIN] Add tests to check if ConstantFolding can handle null literals and remove the unneeded rules from NullPropagation. c4eea67 [Takuya UESHIN] Fix toString methods. 23e2363 [Takuya UESHIN] Make CaseConversionExpressions foldable if the child is foldable. 0ff7568 [Takuya UESHIN] Add tests for collapsing case statements. 3977d80 [Takuya UESHIN] Add optimization for CaseConversionExpression's. (cherry picked from commit 9a2448d) Signed-off-by: Michael Armbrust <michael@databricks.com>

marmbrus · 2014-06-12T00:59:34Z

Thanks!

Merged into master and 1.0.

Add optimization for `CaseConversionExpression`'s. Author: Takuya UESHIN <ueshin@happy-camper.st> Closes apache#990 from ueshin/issues/SPARK-2052 and squashes the following commits: 2568666 [Takuya UESHIN] Move some rules back. dde7ede [Takuya UESHIN] Add tests to check if ConstantFolding can handle null literals and remove the unneeded rules from NullPropagation. c4eea67 [Takuya UESHIN] Fix toString methods. 23e2363 [Takuya UESHIN] Make CaseConversionExpressions foldable if the child is foldable. 0ff7568 [Takuya UESHIN] Add tests for collapsing case statements. 3977d80 [Takuya UESHIN] Add optimization for CaseConversionExpression's.

* LimitPushDownThroughWindow * fix * fix

Co-authored-by: Egor Krivokon <>

rxin reviewed Jun 9, 2014
View reviewed changes

marmbrus reviewed Jun 9, 2014
View reviewed changes

marmbrus reviewed Jun 11, 2014
View reviewed changes

ueshin added 2 commits June 12, 2014 08:21

Add optimization for CaseConversionExpression's.

3977d80

Add tests for collapsing case statements.

0ff7568

ueshin added 4 commits June 12, 2014 08:21

Make CaseConversionExpressions foldable if the child is foldable.

23e2363

Fix toString methods.

c4eea67

Add tests to check if ConstantFolding can handle null literals and re…

dde7ede

…move the unneeded rules from NullPropagation.

Move some rules back.

2568666

asfgit closed this in 9a2448d Jun 12, 2014

wangyum added a commit that referenced this pull request May 26, 2023

[CARMEL-6057] Limit push down through window (#990)

2b932c0

* LimitPushDownThroughWindow * fix * fix

udaynpusa pushed a commit to mapr/spark that referenced this pull request Jan 30, 2024

[EZSPA-663] Spark-3.2.0 apps submission fails (apache#990)

0c7f32d

Co-authored-by: Egor Krivokon <>

mapr-devops pushed a commit to mapr/spark that referenced this pull request May 8, 2025

[EZSPA-663] Spark-3.2.0 apps submission fails (apache#990)

8bfdcd8

Co-authored-by: Egor Krivokon <>

Conversation

ueshin commented Jun 6, 2014

Uh oh!

AmplabJenkins commented Jun 6, 2014

Uh oh!

AmplabJenkins commented Jun 6, 2014

Uh oh!

AmplabJenkins commented Jun 6, 2014

Uh oh!

AmplabJenkins commented Jun 6, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rxin commented Jun 9, 2014

Uh oh!

ueshin commented Jun 9, 2014

Uh oh!

AmplabJenkins commented Jun 9, 2014

Uh oh!

AmplabJenkins commented Jun 9, 2014

Uh oh!

AmplabJenkins commented Jun 9, 2014

Uh oh!

AmplabJenkins commented Jun 9, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AmplabJenkins commented Jun 10, 2014

Uh oh!

AmplabJenkins commented Jun 10, 2014

Uh oh!

AmplabJenkins commented Jun 10, 2014

Uh oh!

AmplabJenkins commented Jun 10, 2014

Uh oh!

AmplabJenkins commented Jun 10, 2014

Uh oh!

ueshin commented Jun 10, 2014

Uh oh!

AmplabJenkins commented Jun 10, 2014

Uh oh!

AmplabJenkins commented Jun 10, 2014

Uh oh!

AmplabJenkins commented Jun 10, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AmplabJenkins commented Jun 11, 2014

Uh oh!

AmplabJenkins commented Jun 11, 2014

Uh oh!

AmplabJenkins commented Jun 11, 2014

Uh oh!

AmplabJenkins commented Jun 11, 2014

Uh oh!

marmbrus commented Jun 11, 2014

Uh oh!

ueshin commented Jun 11, 2014

Uh oh!

AmplabJenkins commented Jun 11, 2014

Uh oh!

AmplabJenkins commented Jun 11, 2014

Uh oh!

AmplabJenkins commented Jun 12, 2014